Skip to content

India - Water#40

Open
indiakato wants to merge 9 commits intoAda-C14:masterfrom
indiakato:master
Open

India - Water#40
indiakato wants to merge 9 commits intoAda-C14:masterfrom
indiakato:master

Conversation

@indiakato
Copy link

Task List

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
Describe in your own words what the Model is doing in Rails The model is creating a database and allowing you to edit, change etc that database while running the server
Describe in your own words what the Controller is doing in Rails The controller is the "middle man" between the model and views. It allows the views to access the database
Describe in your own words what the View is doing in Rails The view is allowing the data from the model to be viewed on the page. It can access the data through the controller via instance variables.
Describe an edge-case controller test you wrote An edge case I wrote was for updating a task with an invalid id. It tested to make sure that when you tried to edit a task that doesn't exist, it will redirect you back to the root path as well as does not change the task count
What is the purpose of using strong params? (i.e. the params method in the controller) Strong params protects the attributes and prevents a forbidden attributes error
How are Rails migrations related to Rails models? rails migrations change the rails models by updating the database
Describe one area of Rails that are still unclear on I'm still a little unclear on testing, more specifically how these tests are calling each of the methods. I was unable to get one of my must_respond_with :redirect tests to pass without an error and I'm still unsure why this is the case.

Copy link

@beccaelenzil beccaelenzil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Task List

Major Learning Goals/Code Review

Criteria yes/no, and optionally any details/lines of code to reference
At least 6 commits with meaningful commit messages ✔️
Routes follow RESTful conventions ✔️
Uses named routes (like _path) ✔️
Creates Models and migrations ✔️
Creates styled views Not required.
Handles errors like nonexistant tasks ✔️
Uses form_with to render forms in Rails ✔️

Functional Requirements/Manual Testing

Functional Requirement yes/no
Successfully handles index & show ✔️
index & show tests pass ✔️
Successfully handles: New, Create ✔️
New, Create tests pass ✔️
Successfully handles: Edit, Update ✔️
Edit, Update tests pass with valid & invalid task ids ✔️
Successfully handles: Destroy, Task Complete See inline comments
Tests for Destroy & Task Complete include tests for valid and invalid task ids See inline comments

Overall Feedback

Good work over all on task list. You created your first web app using rails and did a great job following rails conventions. I've left a few inline comments for you to review. In your reflection question you said you were still unsure about testing so I including a description of what you should test for the complete method. On your next project, you will have the opportunity to work with partial views, strong params, and styles. Keep up the hard work!

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 5+ in Code Review && 6+ in Functional Requirements ✔️
Yellow (Approaches Standards) 3+ in Code Review && 5+ in Functional Requirements, or the instructor judges that this project needs special attention
Red (Not at Standard) 0-2 in Code Review or 0-4 in Functional Reqs, or assignment is breaking/doesn’t run with less than 5 minutes of debugging, or the instructor judges that this project needs special attention

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Perfect Indentation
Descriptive/Readable
Logical/Organized

return
end

def complete

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only to unmark a task complete is through the edit action. You should add this functionality to a custom method. Consider how making two separate methods, for instance mark_complete and mark_incomplete would make this action idempotent.

Comment on lines +10 to +11
<%= f.label :completed_at %>
<%= f.text_field :completed_at %>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider whether this should be entered by the user. Instead, it would make sense to set this as nil initially, and change to Time.now when it's marked completed (as you do).

@@ -0,0 +1,15 @@
<%= form_with model: @task do |f| %>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the edit and new views would be a great place to use partial views.

end

def create
@task = Task.new(name: params[:task][:name], description: params[:task][:description], completed_at: params[:task][:completed_at])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using strong params to DRY up your code.

Comment on lines +174 to +185
it "can update the time" do
task = Task.new(name: "watch game", description: "watch redzone")

task.save
id = task.id

expect {
patch complete_task_path(id)
}.wont_equal nil

#must_respond_with :redirect
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check that the completed_at attribute has been updated. You should also test for the redirect behavior you expect. Finally, you should also include another test to check for the correct behavior for an invalid id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants